feat: lua script commands triggers - #447
Malaydewangan09 merged 2 commits into
Conversation
|
This PR doesn't contain any Lua. The branch is Zero files under Separately, this diff is byte-identical to #446 (both 1950 lines), and #438 is the R subset of both. So #438, #446 and #447 are three PRs carrying overlapping copies of the same commits rather than one module each. Merging any one of them will conflict the other two. Could you rebase each onto current main with only its own module: #438 R, #446 Perl, and this one Lua? I'll review them properly once they're separated. Nothing here reflects on the code itself, I just can't review Lua triggers that aren't in the diff. |
|
Hey @Abhishek84313 👋, are there any updates on this? |
Haven't checked it, i will let you know |
Adds ScriptTrigger and CommandsTrigger to plugin-script-lua, one of the
submodules still missing them. Both live in io.kestra.plugin.scripts.lua,
alongside the existing Script and Commands tasks.
ScriptTrigger polls by running an inline Lua script through the Script
task; CommandsTrigger does the same for a list of commands through the
Commands task. Each emits an execution when exitCondition matches, which
accepts either "exit N" (compared against the process exit code) or a
regex, falling back to a substring match, against vars emitted via the
::{"outputs":{...}}:: convention.
Edge state is kept in the namespace KV store rather than in an instance
field, since a polling trigger is rebuilt from the flow definition and
serialized to a worker on every poll. The implementation follows the
reviewed plugin-script-r triggers.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
61ad5d1 to
714d81d
Compare
|
Thanks for catching this @Malaydewangan09, you were right on all counts. The cause was a one-character branch name mix-up on my side: the actual Lua I've rebased this branch onto current main. It is now a single commit
No Since R (#438) and Perl (#446) are already merged, those two are resolved and While rebasing I noticed my Lua code predated the review feedback on #438, so
On the issue's proposed |
|
Thanks, this is the right diff now. Only Ran it on current main with Docker: 49/49 pass, including the container-backed Two things missing that the R and Perl PRs did include:
One optional: Happy to approve once the two docs bits are in. |
|
Two more from a closer pass, both small:
|
- List CommandsTrigger and ScriptTrigger under plugin-script-lua in AGENTS.md - Add a Triggers section to the Lua plugin doc - Fix exitCondition docs: the regex is matched against emitted vars only, logs are never read, so only 'exit N' can match a failed run - Add ScriptTriggerEvaluateTest driving ScriptTrigger.evaluate() against a real nickblah/lua container (exit code, emitted vars, no match, edge mode) - Only write the edge state to KV when the result changes, as in the PowerShell triggers, with matching EdgeStateTest coverage Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks @Malaydewangan09, all addressed in 8521610: AGENTS.md: added CommandsTrigger and ScriptTrigger under plugin-script-lua. |
|
All four addressed in |
What changes are being made and why?
Adds
ScriptTriggerandCommandsTriggertoplugin-script-lua, which wasone of the submodules still missing them.
Part of #313, closes #428.
Both triggers live in
io.kestra.plugin.scripts.lua— the same package as theScriptandCommandstasks, with no nested subpackage — matching theexisting implementations in
plugin-script-r,plugin-script-perl,plugin-script-go,plugin-script-node,plugin-script-python,plugin-script-rubyandplugin-script-shell.ScriptTriggerpolls by running an inline Lua script through theScripttask and emits an execution when the condition matches.
CommandsTriggerdoes the same for a list of commands through the
Commandstask.Each trigger exposes:
containerImagenickblah/luascript/commandsexitConditionintervalPT60SedgetrueexitConditionaccepts eitherexit N, which compares against the processexit code, or any other string, which is treated as a regex (falling back to a
substring match if the regex is invalid) against vars emitted via the
::{"outputs":{...}}::convention. A blankexitConditionis rejected ratherthan silently matching everything.
When
edgeis enabled the previous result is kept in the namespace KV store,keyed by flow and trigger id, rather than in an instance field — a polling
trigger is rebuilt from the flow definition and serialized to a worker on every
poll, so in-memory state would not survive between evaluations.
The trigger output carries
timestamp,condition,exitCodeandvars;execution id, namespace and flow id come from the standard
TriggerService.generateExecutioncontext. A failed evaluation is logged andreturns empty rather than propagating, so a broken script cannot block the
scheduler.
The implementation follows the reviewed
plugin-script-rtriggers.How the changes have been QAed?
Unit and condition-matching tests were added for both triggers
(
ScriptTriggerTest,CommandsTriggerTest,ScriptTriggerConditionTest,CommandsTriggerConditionTest,EdgeStateTest), covering exit-codeconditions, regex and substring matching against emitted vars, empty/null
conditions, edge-mode transition behaviour across fresh trigger instances, and
KV key scoping between flows and triggers.
41 tests that do not require Docker pass locally, and
compileJavaandcompileTestJavaboth succeed. The Docker-backed cases inCommandsTriggerTestcould not be run on the development machine, which has noDocker available — they need a CI run to confirm. They use
lua -e 'os.exit(1)'/os.exit(0)againstnickblah/lua:latest, consistentwith the existing
CommandsTestand theall_lua.yamlsanity check thatalready invoke
luain that image.Trigger on a failing script: